Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: add e2e prover #188

Merged
merged 22 commits into from
Sep 11, 2024
Merged

Feat: add e2e prover #188

merged 22 commits into from
Sep 11, 2024

Conversation

kunxian-xia
Copy link
Collaborator

@kunxian-xia kunxian-xia commented Sep 4, 2024

Fixes #96.

  • We define types like ZKVMProvingKey, ZKVMVerifyingKey, ZKVMConstraintSystem.
  • We requires that the set of logup sum in opcode/table circuits sum to zero.
  • The critical requirement that $\prod \textrm{proof}.r_{out} = \prod \textrm{proof}.w_{out}$ introduced in the code is not enabled for now. This will be turned on once we have finished cpu state init / finalize #127 and memory init / finalize #126.
  • We also made some minor changes to ceno_emul crate to store Instruction inside step record.

@kunxian-xia kunxian-xia marked this pull request as draft September 4, 2024 12:56
@kunxian-xia kunxian-xia self-assigned this Sep 4, 2024
@kunxian-xia kunxian-xia marked this pull request as ready for review September 5, 2024 15:51
@kunxian-xia
Copy link
Collaborator Author

This pr is still buggy and not ready for review. Just make it to trigger CI for later commits.

@kunxian-xia
Copy link
Collaborator Author

kunxian-xia commented Sep 9, 2024

  • Needs to test the case in which num_instance is not a power of two.

Copy link
Collaborator

@hero78119 hero78119 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The design looks super nice and barely just a few improvement feedback

ceno_zkvm/src/scheme/tests.rs Outdated Show resolved Hide resolved
ceno_zkvm/src/tables/range.rs Outdated Show resolved Hide resolved
ceno_zkvm/src/tables/range.rs Show resolved Hide resolved
ceno_zkvm/src/tables/range.rs Outdated Show resolved Hide resolved
ceno_zkvm/src/tables/range.rs Show resolved Hide resolved
ceno_zkvm/examples/riscv_add.rs Outdated Show resolved Hide resolved
ceno_zkvm/examples/riscv_add.rs Outdated Show resolved Hide resolved
@kunxian-xia kunxian-xia requested review from hero78119, dreamATD and spherel and removed request for spherel September 10, 2024 03:26
Copy link
Collaborator

@hero78119 hero78119 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! A well-thought-out and well refined design 👍 👍

let mut zkvm_witness = ZKVMWitnesses::default();
// assign opcode circuits
zkvm_witness
.assign_opcode_circuit::<AddInstruction<E>>(&zkvm_cs, &add_config, records)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Future TODO)
Just an further refine of api , if we make instruction config implementing a trait, then we can also keep xxx_config in zkvm_cs. With that, the api can be further simplified to

zkvm_cs.register_opcode_circuit::<AddInstruction<E>>(); // no return value, config in maintained in zkvm_cs
...
zkvm_witness.assign_opcode_circuit::<AddInstruction<E>>(&zkvm_cs, records)
...

@kunxian-xia kunxian-xia merged commit 0cfa10b into master Sep 11, 2024
4 checks passed
@kunxian-xia kunxian-xia deleted the feat/batched_zkvm_prover branch September 11, 2024 05:42
kunxian-xia pushed a commit that referenced this pull request Sep 12, 2024
…212)

_Follow-up to #188

The above PR started to depend on a special internal type, originally
and confusingly named `Instruction`.

This PR uses the intended public type `DecodedInstruction` and makes the
weird one private.

Co-authored-by: Aurélien Nicolas <[email protected]>
@kunxian-xia kunxian-xia mentioned this pull request Sep 18, 2024
hero78119 pushed a commit that referenced this pull request Sep 30, 2024
hero78119 pushed a commit that referenced this pull request Sep 30, 2024
…212)

_Follow-up to #188

The above PR started to depend on a special internal type, originally
and confusingly named `Instruction`.

This PR uses the intended public type `DecodedInstruction` and makes the
weird one private.

Co-authored-by: Aurélien Nicolas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

implement batched opcodes constraint system & prover/verifier
2 participants